do you have a scenario in mind where you would set this False with your current operator?

yes. When the unsupportedConfigOverrides have values in them.

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, soltysh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Change the godoc as per my comment.

The current use cases described for this are:

1. kube-apiserver wants to prevent an update to a new payload (micro) because user has set unsupported changes
2. CLI and UI wants to display information about failing or non-upgradable ClusterOperators

There are a few future use cases we need to describe:

1. The MCO needs to block 4.Y updates of kube-apiserver when kubelets older than a specific version
2. We want to prevent an upgrade based on a specific API condition

@abhinavdahiya and I had a long chat, I think what we allow right now is:

1. David can set upgradeable false for unsupported version config
2. CVO will write Upgradeable=False to CV if any operator is failing or if anyone has Upgradeable false and a message
3. CLI and UI will show upgradeable false as informational message
4. CVO will not halt on upgradeable false yet

Once we have that much implemented, we'll go back and assess.

Future CVO might have "precondition jobs" that we use to assess whether a version could be started (without actually updating anything). This would capture most of the "quick and dirty checks", assuming we don't have many of them.

In the short term, the CVO will do the following

1. Versions takes priority - we will never go past a CO that has cluster operator status versions that don't match
2. If versions match, Available=False and Failing=True will both block CVO from going past
3. If no versions are set, in the short term, Progressing=True will also block CVO (but when versions are set Progressing = true is informational)

Operators should be Available=True if they satisfy the "reason they are part of the payload" - i.e. if they expose an API, if the API is deployed they are available true. If they have a single operand that should always be running, they are Available=true. If they have an optional (as defined by biz use case) operand, they are available "if they have either created the operand or are waiting on a human". If they have many operands, they should become available when "the default operands as defined by a human" are created.

Operators should be Available=True if they satisfy the "reason they are part of the payload" - i.e. if they expose an API, if the API is deployed they are available true. If they have a single operand that should always be running, they are Available=true. If they have an optional (as defined by biz use case) operand, they are available "if they have either created the operand or are waiting on a human". If they have many operands, they should become available when "the default operands as defined by a human" are created.

So applying this statement to the two devex use cases:

1. the registry operand should be considered "optional" (your cluster can work without it), thus on platforms where the registry cannot deploy (no storage), the registry operator would still report available=true/progressing=false/failing=false (assuming the operator itself is otherwise happy). This makes the clusteroperator conditions largely useless for an admin checking the health of the registry, but ensures registry will not block install or upgrade.

2. the samples operand should also be considered "optional", meaning again, the samples operator should just report "available=true/progressing=false/failing=false" even if it has not created any samples yet.

For both of these, we will want to continue to have actually useful conditions on the operator-specific resource that indicate the state of the operand and why. Unfortunately this means there's no consistent way to check that all operators and their operands are healthy/stable. Or at least that the ClusterOperator resource is not that mechanism.

I think this largely goes against the current ClusterOperator api documentation(so that should be fixed) and from the perspective of an operator author trying to report useful status about the state of my operand, I think it's a step backwards, but if we are saying the CVO isn't going to switch to a different mechanism for determining when an operator is sufficiently stable such that the install/upgrade can continue, I guess that's what we have.

/cc @openshift/sig-developer-experience

1. This makes the clusteroperator conditions largely useless for an admin checking the health of the registry, but ensures registry will not block install or upgrade.

Why? If you don't have an installed registry your health is "good".

2. the samples operand should also be considered "optional", meaning again, the samples operator should just report "available=true/progressing=false/failing=false" even if it has not created any samples yet.

Why? The specific condition of you don't have a pull secret to the registry is something the operator should report but requires human intervention (today). When we ship, it will again be an error.

Why? If you don't have an installed registry your health is "good".

And when i do have an installed registry but the storage isn't configured/available, but the operator still reports available=true/progressing=false so that it doesn't block the install? I wouldn't consider that a registry that's in good health.

Why? The specific condition of you don't have a pull secret to the registry is something the operator should report but requires human intervention (today). When we ship, it will again be an error.

Because importing the samples takes time(thus making the install take longer) and having the samples available isn't critical to having a working cluster. We're being asked why we're delaying the install.

And when i do have an installed registry but the storage isn't configured/available, but the operator still reports available=true/progressing=false so that it doesn't block the install?

There is no difference between a registry without configured storage and NetworkOperator Type=None - if you are waiting for human, you are available.

There is no difference between a registry without configured storage and NetworkOperator Type=None - if you are waiting for human, you are available.

That seems like a slippery slope. When no registry pods can be scheduled because of node issues that require a human to go add more capacity, am I available/healthy? When the storage is configured to a PVC but the autoprovisioner didn't generate a PV that's claimable, am I healthy because the admin just needs to go create a PV for me?

If anything it seems particularly weird to report available=true when you know the admin needs to take some action to actually make the thing available.

From an admin/end-user perspective, it doesn't matter why the registry isn't running, I need to know that it's not(and then i will care why and see if i need to do something about it). Clearly the clusteroperator "available=true" condition isn't the thing that tells the admin that.

When no registry pods can be scheduled because of node issues that require a human to go add more capacity, am I available/healthy?

No, you aren't.

When the storage is configured to a PVC but the autoprovisioner didn't generate a PV that's claimable, am I healthy because the admin just needs to go create a PV for me?

No you aren't.

Those both seemed pretty easy?

Those both seemed pretty easy?

I don't think they look that different to an admin. The difference between "you didn't configure storage at all" and "you configured storage but it's not available, maybe you did it wrong" is pretty subtle, yet you're saying in one case the registry is "available" and in the other it's not. Not configuring storage is basically a special case of "you configured storage that isn't usable".

And your criteria above was "if you're waiting for a human you're available". Both of the scenarios I presented are cases where you're also waiting for a human. So I don't see it as an easy distinction, but more importantly I don't see it as a useful distinction for an administrator who just cares if the thing is actually push/pullable or not (or upgradeable or not).

Which gets to the larger point. There are two concerns:

1. reporting availability of the operand (clearly available=true isn't for that)

2. determining when an upgrade is done/can be started: the operator has to decide, given the state of itself and its operand, whether it is ok with an upgrade proceeding. For the registry operator, I probably don't care why there are no registry pods running. Either i'm going to allow you to upgrade when there are no registry pods, or i'm never going to allow you to upgrade when there are no registry pods. But i'm not going to say "when you haven't configured storage, you can upgrade, but when you've configured invalid storage, i'm not going to let you upgrade" because it makes no difference in what happens during the upgrade. I might want to block an upgrade when you've configured valid storage but no registry pods are running because I want to ensure storage migration happens correctly (then again I might not if i know it doesn't matter since I think we've only had one actual storage format migration in the history of the registry).

It looks like most of the action in this pull is about the meaning of Available, not the meaning of Upgradeable.

@smarterclayton doc updated

doc may need clarification to indicate that only False does something

New changes are detected. LGTM label has been removed.

I'm late to the game, but I'll add a little more detail to what makes this confusing. I'm working on the console-operator and deciding how to set the statuses Available, Progressing, Failing when ManagementState:Removed.

• If removed, I am deleting all of my resources, including the deployment for my operand (console).
• If my operand does not exist, logically Available:False is the correct reporting (to a human). Its not possible to use it, it doesn't exist.
• Progressing:false and Failing:false are not hard to reason about here. While I am deleting resources, Progressing: true. So long as I succeed, Failing:false.

It seems very confusing that I should actually be reporting Available: true on my operand, as its supposed to be deleted by user intent, even though it is clearly unavailable.

Gonna add one more.

If I set my operator ManagementState: Unmanaged, I desire to set Available: operatorsv1.ConditionUnknown because the operator doesn't know what a user might be doing. At present my understanding is that I am still to set it to Available: true because the operator is honoring the user's desired state. I'm not sure the intention of ConditionUnknown at this point.

However this: https://github.com/openshift/library-go/blob/master/pkg/operator/status/status_controller.go#L116

// if we are unmanaged, force everything to Unknown.
if detailedSpec.ManagementState == operatorv1.Unmanaged {
    clusterOperatorObj.Status = configv1.ClusterOperatorStatus{}
    configv1helpers.SetStatusCondition(&clusterOperatorObj.Status.Conditions, configv1.ClusterOperatorStatusCondition{Type: configv1.OperatorAvailable, Status: configv1.ConditionUnknown, Reason: "Unmanaged"})
    configv1helpers.SetStatusCondition(&clusterOperatorObj.Status.Conditions, configv1.ClusterOperatorStatusCondition{Type: configv1.OperatorProgressing, Status: configv1.ConditionUnknown, Reason: "Unmanaged"})
    configv1helpers.SetStatusCondition(&clusterOperatorObj.Status.Conditions, configv1.ClusterOperatorStatusCondition{Type: configv1.OperatorFailing, Status: configv1.ConditionUnknown, Reason: "Unmanaged"})
    configv1helpers.SetStatusCondition(&clusterOperatorObj.Status.Conditions, configv1.ClusterOperatorStatusCondition{Type: configv1.OperatorUpgradeable, Status: configv1.ConditionUnknown, Reason: "Unmanaged"})
}

makes me question what I previously stated.